Skip to content

Conversation

pietroalbini
Copy link
Member

This PR updates remote-test-server to add two new flags:

  • --sequential disables parallel test execution, accepting one connection at the time instead. We need this for Ferrocene as one of our emulators occasionally deadlocks when running multiple tests in parallel.
  • --bind <ip:port> allows customizing the IP and port remote-test-server binds to, rather than using the default value.

While I was changing the flags, and after chatting on what to do on Zulip, I took this opportunity to cleanup argument handling in remote-test-server, which is a breaking change:

  • The verbose argument has been renamed to the --verbose flag.
  • The remote argument has been removed in favor of the --bind 0.0.0.0:12345 flag. The only thing the argument did was to change the bound IP to 0.0.0.0, which can easily be replicated with --bind and also is not secure as our "remote" default.

I'm also open to keep the old arguments with deprecation warnings.

r? @Mark-Simulacrum

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Sep 23, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 23, 2022
@pietroalbini pietroalbini marked this pull request as draft September 23, 2022 13:55
@pietroalbini pietroalbini added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 23, 2022
@pietroalbini
Copy link
Member Author

Moving this to draft as I need to also update the in-tree places where we call remote-test-server.

@pietroalbini
Copy link
Member Author

There doesn't seem to be anything in our repository using those flags from a cursory glance.

@pietroalbini pietroalbini marked this pull request as ready for review September 23, 2022 14:19
@pietroalbini pietroalbini added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 23, 2022
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=iffy

@bors
Copy link
Collaborator

bors commented Oct 1, 2022

📌 Commit 20638eb has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 1, 2022
@bors
Copy link
Collaborator

bors commented Oct 1, 2022

⌛ Testing commit 20638eb with merge b34cff1...

@bors
Copy link
Collaborator

bors commented Oct 2, 2022

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing b34cff1 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 2, 2022
@bors bors merged commit b34cff1 into rust-lang:master Oct 2, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 2, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (b34cff1): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.9% [2.2%, 3.3%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

Footnotes

  1. the arithmetic mean of the percent change

  2. number of relevant changes

@pietroalbini pietroalbini deleted the pa-remote-test-server-improvements branch October 2, 2022 08:33
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
…ovements, r=Mark-Simulacrum

Change argument handling in `remote-test-server` and add new flags

This PR updates `remote-test-server` to add two new flags:

* `--sequential` disables parallel test execution, accepting one connection at the time instead. We need this for Ferrocene as one of our emulators occasionally deadlocks when running multiple tests in parallel.
* `--bind <ip:port>` allows customizing the IP and port `remote-test-server` binds to, rather than using the default value.

While I was changing the flags, and [after chatting on what to do on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/remote-test-server.20flags),  I took this opportunity to cleanup argument handling in `remote-test-server`, which is a breaking change:

* The `verbose` argument has been renamed to the `--verbose` flag.
* The `remote` argument has been removed in favor of the `--bind 0.0.0.0:12345` flag. The only thing the argument did was to change the bound IP to 0.0.0.0, which can easily be replicated with `--bind` and also is not secure as our "remote" default.

I'm also open to keep the old arguments with deprecation warnings.

r? `@Mark-Simulacrum`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants